feat(expo): re-introduce two-way JS/native session sync#8088
Open
chriscanin wants to merge 3 commits intomainfrom
Open
feat(expo): re-introduce two-way JS/native session sync#8088chriscanin wants to merge 3 commits intomainfrom
chriscanin wants to merge 3 commits intomainfrom
Conversation
Re-applies the changes from #8032 which was reverted in #8065. This PR exists for visibility and review before re-merging. Original changes: - Two-way JS/native token sync for expo native components - Native session cleared on sign-out - Improved initialization error handling with timeout/failure messages - Additional debug logging in development
🦋 Changeset detectedLatest commit: 5b3a88f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
📝 WalkthroughWalkthroughThis PR reintroduces two-way JS/native session synchronization for Expo. Changes: bump Android API version to 1.0.9; refactor ClerkExpoModule to guard initialization, add timeouts and token-update flows; add getClientToken and signOut methods in ClerkViewFactory and expose native keychain-backed token management; implement keychain-backed token sync and session gating in iOS templates; add NativeSessionSync to the React provider; and update UserButton and useUserProfileModal to sync tokens and conditionally handle sign-out. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/expo/src/hooks/useUserProfileModal.ts (1)
61-127:⚠️ Potential issue | 🟠 MajorAdd regression coverage before reintroducing this flow.
This hook now depends on a multi-step contract across JS, the native module, and native UI (
getSession→configure→ modal presentation → post-dismiss sign-out reconciliation), but the PR adds no automated coverage for it. Since this exact auth-sync work was already reverted once, we need regression tests around token sync and native-driven sign-out before merge.As per coding guidelines, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/hooks/useUserProfileModal.ts` around lines 61 - 127, The new presentUserProfile flow (presentUserProfile hook) requires regression tests to cover the JS↔native token sync and native-driven sign-out reconciliation; add tests that mock ClerkExpo.getSession, ClerkExpo.configure, ClerkExpo.presentUserProfile, ClerkExpo.signOut, tokenCache.getToken, and the clerk.signOut method to assert: (1) when native has no session but tokenCache.getToken returns a bearer token, ClerkExpo.configure is called with clerk.publishableKey and that token and presentUserProfile is invoked; (2) when post-modal ClerkExpo.getSession returns null but hadNativeSessionBefore was true, clerk.signOut (and ClerkExpo.signOut if present) are called; and (3) the inverse case where configure produces a native session avoids JS signOut; target tests at the unit/integration layer exercising presentUserProfile (the useUserProfileModal hook) and use Jest mocks/spies for ClerkExpo.getSession, configure, presentUserProfile, signOut and tokenCache.getToken to verify the exact call sequence and side effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt`:
- Around line 253-257: The signOut(promise: Promise) branch that returns when
!Clerk.isInitialized currently resolves without clearing the stored device
token; update signOut to always remove the stored DEVICE_TOKEN from
SharedPreferences (the same key written by configure()) before resolving, even
when Clerk.isInitialized is false. Locate the signOut method in
ClerkExpoModule.kt and add logic to obtain the module's SharedPreferences and
remove the "DEVICE_TOKEN" entry (or the constant used for that key), then
proceed to promise.resolve(null).
In `@packages/expo/ios/ClerkExpoModule.swift`:
- Around line 23-26: The template is missing the required protocol method
getClientToken() which breaks conformance to ClerkViewFactoryProtocol; add a
concrete implementation of getClientToken() inside the ClerkViewFactory class
(the type that implements ClerkViewFactoryProtocol) that returns the correct
client token string or nil as appropriate, ensuring the method signature exactly
matches getClientToken() -> String? and is public/internal consistent with other
protocol methods; update any backing storage or token retrieval logic used by
configure/getSession to return the same token value.
In `@packages/expo/ios/templates/ClerkViewFactory.swift`:
- Around line 323-330: The signOut() method currently returns early when there
is no live native session, skipping Clerk.clearAllKeychainItems() and
Self.clerkConfigured = false; change the control flow in signOut() so that
clearing native keychain state and resetting Self.clerkConfigured always runs
regardless of whether Clerk.shared.session?.id is present—attempt the signOut
call only if sessionId exists (try await Clerk.shared.auth.signOut(sessionId:
sessionId)), but move Clerk.clearAllKeychainItems() and Self.clerkConfigured =
false outside/after that guard/conditional so they execute unconditionally;
update the signOut() function in ClerkViewFactory.swift accordingly.
---
Outside diff comments:
In `@packages/expo/src/hooks/useUserProfileModal.ts`:
- Around line 61-127: The new presentUserProfile flow (presentUserProfile hook)
requires regression tests to cover the JS↔native token sync and native-driven
sign-out reconciliation; add tests that mock ClerkExpo.getSession,
ClerkExpo.configure, ClerkExpo.presentUserProfile, ClerkExpo.signOut,
tokenCache.getToken, and the clerk.signOut method to assert: (1) when native has
no session but tokenCache.getToken returns a bearer token, ClerkExpo.configure
is called with clerk.publishableKey and that token and presentUserProfile is
invoked; (2) when post-modal ClerkExpo.getSession returns null but
hadNativeSessionBefore was true, clerk.signOut (and ClerkExpo.signOut if
present) are called; and (3) the inverse case where configure produces a native
session avoids JS signOut; target tests at the unit/integration layer exercising
presentUserProfile (the useUserProfileModal hook) and use Jest mocks/spies for
ClerkExpo.getSession, configure, presentUserProfile, signOut and
tokenCache.getToken to verify the exact call sequence and side effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2579af5-b567-4b39-af9d-c538c6937b51
📒 Files selected for processing (9)
.changeset/native-session-sync-v2.mdpackages/expo/android/build.gradlepackages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.ktpackages/expo/ios/ClerkExpoModule.swiftpackages/expo/ios/ClerkViewFactory.swiftpackages/expo/ios/templates/ClerkViewFactory.swiftpackages/expo/src/hooks/useUserProfileModal.tspackages/expo/src/native/UserButton.tsxpackages/expo/src/provider/ClerkProvider.tsx
Comment on lines
+71
to
106
| if (!Clerk.isInitialized.value) { | ||
| // First-time initialization — write the bearer token to SharedPreferences | ||
| // before initializing so the SDK boots with the correct client. | ||
| if (!bearerToken.isNullOrEmpty()) { | ||
| reactApplicationContext.getSharedPreferences("clerk_preferences", Context.MODE_PRIVATE) | ||
| .edit() | ||
| .putString("DEVICE_TOKEN", bearerToken) | ||
| .apply() | ||
| } | ||
|
|
||
| // Wait for initialization to complete with timeout | ||
| try { | ||
| withTimeout(10_000L) { | ||
| Clerk.isInitialized.first { it } | ||
| Clerk.initialize(reactApplicationContext, pubKey) | ||
|
|
||
| // Wait for initialization to complete with timeout | ||
| try { | ||
| withTimeout(10_000L) { | ||
| Clerk.isInitialized.first { it } | ||
| } | ||
| } catch (e: TimeoutCancellationException) { | ||
| val initError = Clerk.initializationError.value | ||
| val message = if (initError != null) { | ||
| "Clerk initialization timed out: ${initError.message}" | ||
| } else { | ||
| "Clerk initialization timed out after 10 seconds" | ||
| } | ||
| promise.reject("E_TIMEOUT", message) | ||
| return@launch | ||
| } | ||
| } catch (e: TimeoutCancellationException) { | ||
| val initError = Clerk.initializationError.value | ||
| val message = if (initError != null) { | ||
| "Clerk initialization timed out: ${initError.message}" | ||
|
|
||
| // Check for initialization errors | ||
| val error = Clerk.initializationError.value | ||
| if (error != null) { | ||
| promise.reject("E_INIT_FAILED", "Failed to initialize Clerk SDK: ${error.message}") | ||
| } else { | ||
| "Clerk initialization timed out after 10 seconds" | ||
| promise.resolve(null) | ||
| } | ||
| promise.reject("E_TIMEOUT", message) | ||
| return@launch |
Contributor
There was a problem hiding this comment.
Wait for the first token-backed session before resolving configure().
This cold-start branch resolves as soon as Clerk.isInitialized flips. Both packages/expo/src/hooks/useUserProfileModal.ts and packages/expo/src/native/UserButton.tsx immediately call getSession() after configure(), so a first launch with a JS bearer token can still observe null and treat native as unsigned-in. That breaks the sync/sign-out reconciliation this PR is reintroducing.
Suggested fix
try {
withTimeout(10_000L) {
Clerk.isInitialized.first { it }
}
+ if (!bearerToken.isNullOrEmpty()) {
+ withTimeout(5_000L) {
+ Clerk.sessionFlow.first { it != null }
+ }
+ }
} catch (e: TimeoutCancellationException) {
Comment on lines
253
to
257
| override fun signOut(promise: Promise) { | ||
| if (!Clerk.isInitialized.value) { | ||
| promise.reject("E_NOT_INITIALIZED", "Clerk SDK is not initialized. Call configure() first.") | ||
| // Resolve gracefully when not initialized (matches iOS behavior) | ||
| promise.resolve(null) | ||
| return |
Contributor
There was a problem hiding this comment.
Clear DEVICE_TOKEN even when native was never initialized.
ClerkProvider calls ClerkExpo.signOut() on JS-side sign-out. In this branch we resolve without removing the SharedPreferences token that configure() wrote, so the next native init can still boot back into the old client/session.
Suggested fix
override fun signOut(promise: Promise) {
+ val prefs = reactApplicationContext.getSharedPreferences("clerk_preferences", Context.MODE_PRIVATE)
if (!Clerk.isInitialized.value) {
- // Resolve gracefully when not initialized (matches iOS behavior)
+ prefs.edit().remove("DEVICE_TOKEN").apply()
promise.resolve(null)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| override fun signOut(promise: Promise) { | |
| if (!Clerk.isInitialized.value) { | |
| promise.reject("E_NOT_INITIALIZED", "Clerk SDK is not initialized. Call configure() first.") | |
| // Resolve gracefully when not initialized (matches iOS behavior) | |
| promise.resolve(null) | |
| return | |
| override fun signOut(promise: Promise) { | |
| val prefs = reactApplicationContext.getSharedPreferences("clerk_preferences", Context.MODE_PRIVATE) | |
| if (!Clerk.isInitialized.value) { | |
| prefs.edit().remove("DEVICE_TOKEN").apply() | |
| promise.resolve(null) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/expo/android/src/main/java/expo/modules/clerk/ClerkExpoModule.kt`
around lines 253 - 257, The signOut(promise: Promise) branch that returns when
!Clerk.isInitialized currently resolves without clearing the stored device
token; update signOut to always remove the stored DEVICE_TOKEN from
SharedPreferences (the same key written by configure()) before resolving, even
when Clerk.isInitialized is false. Locate the signOut method in
ClerkExpoModule.kt and add logic to obtain the module's SharedPreferences and
remove the "DEVICE_TOKEN" entry (or the constant used for that key), then
proceed to promise.resolve(null).
Comment on lines
23
to
26
| func configure(publishableKey: String, bearerToken: String?) async throws | ||
| func getSession() async -> [String: Any]? | ||
| func getClientToken() -> String? | ||
| func signOut() async throws |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Protocol requirements:"
sed -n '13,27p' packages/expo/ios/ClerkExpoModule.swift
echo
echo "Runtime factory methods:"
rg -n 'func (getClientToken|getSession|signOut)\b' packages/expo/ios/ClerkViewFactory.swift
echo
echo "Template factory methods:"
rg -n 'func (getClientToken|getSession|signOut)\b' packages/expo/ios/templates/ClerkViewFactory.swiftRepository: clerk/javascript
Length of output: 1287
The injected template is missing a required protocol method.
ClerkViewFactoryProtocol requires getClientToken(), but packages/expo/ios/templates/ClerkViewFactory.swift does not implement it. Any app built with this template will fail Swift protocol conformance at compile time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/expo/ios/ClerkExpoModule.swift` around lines 23 - 26, The template
is missing the required protocol method getClientToken() which breaks
conformance to ClerkViewFactoryProtocol; add a concrete implementation of
getClientToken() inside the ClerkViewFactory class (the type that implements
ClerkViewFactoryProtocol) that returns the correct client token string or nil as
appropriate, ensuring the method signature exactly matches getClientToken() ->
String? and is public/internal consistent with other protocol methods; update
any backing storage or token retrieval logic used by configure/getSession to
return the same token value.
Comment on lines
323
to
+330
| public func signOut() async throws { | ||
| guard let sessionId = Clerk.shared.session?.id else { return } | ||
| guard Self.clerkConfigured, let sessionId = Clerk.shared.session?.id else { return } | ||
| try await Clerk.shared.auth.signOut(sessionId: sessionId) | ||
|
|
||
| // Clear all keychain data (device token, cached client/environment, etc.) | ||
| // so the native SDK doesn't boot with a stale token on next launch. | ||
| Clerk.clearAllKeychainItems() | ||
| Self.clerkConfigured = false |
Contributor
There was a problem hiding this comment.
Do not return before clearing native keychain state.
When JS signs out before the native SDK has a live session, this guard exits without deleting the stored device token/cached Clerk state. That leaves stale native auth able to rehydrate on the next open, which is exactly the regression this PR is trying to prevent. packages/expo/ios/ClerkViewFactory.swift has the same guard and needs the same fix.
Suggested fix
`@MainActor`
public func signOut() async throws {
- guard Self.clerkConfigured, let sessionId = Clerk.shared.session?.id else { return }
- try await Clerk.shared.auth.signOut(sessionId: sessionId)
-
- // Clear all keychain data (device token, cached client/environment, etc.)
- // so the native SDK doesn't boot with a stale token on next launch.
- Clerk.clearAllKeychainItems()
- Self.clerkConfigured = false
+ guard Self.clerkConfigured else { return }
+ defer {
+ Clerk.clearAllKeychainItems()
+ Self.clerkConfigured = false
+ }
+
+ if let sessionId = Clerk.shared.session?.id {
+ try await Clerk.shared.auth.signOut(sessionId: sessionId)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public func signOut() async throws { | |
| guard let sessionId = Clerk.shared.session?.id else { return } | |
| guard Self.clerkConfigured, let sessionId = Clerk.shared.session?.id else { return } | |
| try await Clerk.shared.auth.signOut(sessionId: sessionId) | |
| // Clear all keychain data (device token, cached client/environment, etc.) | |
| // so the native SDK doesn't boot with a stale token on next launch. | |
| Clerk.clearAllKeychainItems() | |
| Self.clerkConfigured = false | |
| public func signOut() async throws { | |
| guard Self.clerkConfigured else { return } | |
| defer { | |
| Clerk.clearAllKeychainItems() | |
| Self.clerkConfigured = false | |
| } | |
| if let sessionId = Clerk.shared.session?.id { | |
| try await Clerk.shared.auth.signOut(sessionId: sessionId) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/expo/ios/templates/ClerkViewFactory.swift` around lines 323 - 330,
The signOut() method currently returns early when there is no live native
session, skipping Clerk.clearAllKeychainItems() and Self.clerkConfigured =
false; change the control flow in signOut() so that clearing native keychain
state and resetting Self.clerkConfigured always runs regardless of whether
Clerk.shared.session?.id is present—attempt the signOut call only if sessionId
exists (try await Clerk.shared.auth.signOut(sessionId: sessionId)), but move
Clerk.clearAllKeychainItems() and Self.clerkConfigured = false outside/after
that guard/conditional so they execute unconditionally; update the signOut()
function in ClerkViewFactory.swift accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/expo/src/provider/ClerkProvider.tsx (2)
364-367:⚠️ Potential issue | 🟡 MinorMissing
tokenCachein dependency array.Same issue as above - the effect uses
tokenCache(line 330) for saving native client tokens but doesn't include it in dependencies. This could cause tokens to be saved to a stale cache reference.Suggested fix
- }, [nativeAuthState, clerkInstance]); + }, [nativeAuthState, clerkInstance, tokenCache]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 364 - 367, The effect that calls syncNativeAuthToJs uses tokenCache but doesn't list it in the dependency array, risking saving tokens to a stale cache; update the useEffect (the one that invokes syncNativeAuthToJs) to include tokenCache alongside nativeAuthState and clerkInstance in its dependency array so the effect re-runs when tokenCache changes, ensuring syncNativeAuthToJs always captures the current tokenCache reference.
308-311:⚠️ Potential issue | 🟡 MinorMissing
tokenCachein dependency array.The effect uses
tokenCache(line 192) but it's not included in the dependency array. IftokenCachechanges after mount, the effect will use a stale reference, potentially reading tokens from the wrong cache.Suggested fix
- }, [pk, clerkInstance]); + }, [pk, clerkInstance, tokenCache]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 308 - 311, The effect that sets isMountedRef.current to false in the cleanup references tokenCache elsewhere in the effect but the dependency array only includes pk and clerkInstance; update the effect's dependency array to include tokenCache (or a stable identifier derived from it) so the effect re-runs when tokenCache changes, ensuring the effect does not capture a stale tokenCache; locate the useEffect that returns the cleanup setting isMountedRef.current = false (the closure referencing isMountedRef, pk, clerkInstance, and tokenCache) and add tokenCache to its dependency list (or memoize tokenCache where used) to fix the stale reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/expo/src/provider/ClerkProvider.tsx`:
- Around line 66-136: Add unit/integration tests exercising the
NativeSessionSync component's sync paths: write tests that mount
NativeSessionSync (or the parent ClerkProvider) and simulate JS→native sync by
toggling useAuth isSignedIn, mocking
NativeClerkModule.configure/getSession/signOut and defaultTokenCache.getToken
(use CLERK_CLIENT_JWT_KEY), asserting configure is called with publishableKey
and token and that signOut is invoked on sign-out; also add tests for native→JS
token sync by mocking getSession to return a native session and ensuring
hasSyncedRef prevents redundant configure calls; target tests around
NativeSessionSync, syncToNative, and NativeClerkModule behaviors to cover error
branches and the effectiveTokenCache fallback.
---
Outside diff comments:
In `@packages/expo/src/provider/ClerkProvider.tsx`:
- Around line 364-367: The effect that calls syncNativeAuthToJs uses tokenCache
but doesn't list it in the dependency array, risking saving tokens to a stale
cache; update the useEffect (the one that invokes syncNativeAuthToJs) to include
tokenCache alongside nativeAuthState and clerkInstance in its dependency array
so the effect re-runs when tokenCache changes, ensuring syncNativeAuthToJs
always captures the current tokenCache reference.
- Around line 308-311: The effect that sets isMountedRef.current to false in the
cleanup references tokenCache elsewhere in the effect but the dependency array
only includes pk and clerkInstance; update the effect's dependency array to
include tokenCache (or a stable identifier derived from it) so the effect
re-runs when tokenCache changes, ensuring the effect does not capture a stale
tokenCache; locate the useEffect that returns the cleanup setting
isMountedRef.current = false (the closure referencing isMountedRef, pk,
clerkInstance, and tokenCache) and add tokenCache to its dependency list (or
memoize tokenCache where used) to fix the stale reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: fed743a6-c865-4570-98a6-5768d0a655e0
📒 Files selected for processing (1)
packages/expo/src/provider/ClerkProvider.tsx
Comment on lines
+66
to
+136
| function NativeSessionSync({ | ||
| publishableKey, | ||
| tokenCache, | ||
| }: { | ||
| publishableKey: string; | ||
| tokenCache: TokenCache | undefined; | ||
| }) { | ||
| const { isSignedIn } = useAuth(); | ||
| const hasSyncedRef = useRef(false); | ||
| // Use the provided tokenCache, falling back to the default SecureStore cache | ||
| const effectiveTokenCache = tokenCache ?? defaultTokenCache; | ||
|
|
||
| useEffect(() => { | ||
| if (!isSignedIn) { | ||
| hasSyncedRef.current = false; | ||
|
|
||
| // Clear the native session so native components (UserButton, etc.) | ||
| // don't continue showing a signed-in state after JS-side sign out. | ||
| const ClerkExpo = NativeClerkModule; | ||
| if (ClerkExpo?.signOut) { | ||
| void ClerkExpo.signOut().catch((error: unknown) => { | ||
| if (__DEV__) { | ||
| console.warn('[NativeSessionSync] Failed to clear native session:', error); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (hasSyncedRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| const syncToNative = async () => { | ||
| try { | ||
| const ClerkExpo = NativeClerkModule; | ||
| if (!ClerkExpo?.configure || !ClerkExpo?.getSession) { | ||
| return; | ||
| } | ||
|
|
||
| // Check if native already has a session (e.g. auth via AuthView or initial load) | ||
| const nativeSession = (await ClerkExpo.getSession()) as { | ||
| sessionId?: string; | ||
| session?: { id: string }; | ||
| } | null; | ||
| const hasNativeSession = !!(nativeSession?.sessionId || nativeSession?.session?.id); | ||
|
|
||
| if (hasNativeSession) { | ||
| hasSyncedRef.current = true; | ||
| return; | ||
| } | ||
|
|
||
| // Read the JS SDK's client JWT and push it to the native SDK | ||
| const bearerToken = (await effectiveTokenCache?.getToken(CLERK_CLIENT_JWT_KEY)) ?? null; | ||
| if (bearerToken) { | ||
| await ClerkExpo.configure(publishableKey, bearerToken); | ||
| hasSyncedRef.current = true; | ||
| } | ||
| } catch (error) { | ||
| if (__DEV__) { | ||
| console.warn('[NativeSessionSync] Failed to sync JS session to native:', error); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| void syncToNative(); | ||
| }, [isSignedIn, publishableKey, effectiveTokenCache]); | ||
|
|
||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding tests for the new session sync logic.
The NativeSessionSync component and the two-way sync paths introduce significant new functionality. Tests covering JS→native sync (sign-in/sign-out flows) and native→JS token sync would help ensure this behavior doesn't regress.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/expo/src/provider/ClerkProvider.tsx` around lines 66 - 136, Add
unit/integration tests exercising the NativeSessionSync component's sync paths:
write tests that mount NativeSessionSync (or the parent ClerkProvider) and
simulate JS→native sync by toggling useAuth isSignedIn, mocking
NativeClerkModule.configure/getSession/signOut and defaultTokenCache.getToken
(use CLERK_CLIENT_JWT_KEY), asserting configure is called with publishableKey
and token and that signOut is invoked on sign-out; also add tests for native→JS
token sync by mocking getSession to return a native session and ensuring
hasSyncedRef prevents redundant configure calls; target tests around
NativeSessionSync, syncToNative, and NativeClerkModule behaviors to cover error
branches and the effectiveTokenCache fallback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-applies the changes from #8032 which was reverted in #8065 due to premature merge.
This PR exists for visibility and review before re-merging. The code is identical to the original #8032:
Context
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements